RF: Use scipy.interpolate.BSpline to construct spline basis #393
+33
−31
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #393 +/- ##
==========================================
+ Coverage 80.53% 80.56% +0.03%
==========================================
Files 26 26
Lines 2296 2290 -6
Branches 286 287 +1
==========================================
- Hits 1849 1845 -4
+ Misses 402 401 -1
+ Partials 45 44 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
effigies
force-pushed
the
rf/scipy-bspline-take-3
branch
4 times, most recently
from
6061011 to
d17bd89
Compare
Co-authored-by: Chris Markiewicz <[email protected]>
Use a construction that will potentially allow us to calculate derivatives more easily.
effigies
force-pushed
the
rf/scipy-bspline-take-3
branch
from
d17bd89 to
a22f76c
Compare
effigies
force-pushed
the
rf/scipy-bspline-take-3
branch
from
a22f76c to
58ecbe7
Compare
|
@oesteban This should be a relatively quick review. Apart from the specific B-spline invocation change, the main change is that our design matrices are transposed from before. The effect is that we can stop transposing them after generation. The tests verify that the |
effigies
commented
Sep 27, 2023
|
Read through with fresh eyes. I'm going to go ahead and merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This builds on and replaces #388. The additional padding broke our ability to reconstruct the field, as we were estimating more parameters but not actually saving them.
This PR maintains the current functionality and paves the way for implementing the Jacobian (#391) by using
BSpline()objects directly instead of throughBSpline.design_matrix(). I also restore the use of awithin_supportmask, which significantly decreases the runtime ofunwarp_wf.resamplenodes. There is little effect onbs_filternodes, as we downsample to a low enough resolution to make the fit process very quick already.